bindings/c: implement sqlite3_set_authorizer, sqlite3_db_config, sqlite3_extended_result_codes - #8042
bindings/c: implement sqlite3_set_authorizer, sqlite3_db_config, sqlite3_extended_result_codes#8042glommer wants to merge 1 commit into
Conversation
| /// silent no-op, which is one reason bindings/c refuses | ||
| /// SQLITE_DBCONFIG_DEFENSIVE (the engine is already defensive). If Off | ||
| /// ever becomes supported, the pragma must be gated on a per-connection | ||
| /// defensive flag first (see turso_db_config_int in bindings/c/src/lib.rs). |
| // bindings/c relies on this: it refuses SQLITE_DBCONFIG_DEFENSIVE because | ||
| // the engine is already defensive here. If this ever becomes a real cookie | ||
| // write, it must be gated on a per-connection defensive flag first (see | ||
| // turso_db_config_int in bindings/c/src/lib.rs). |
| /// SQLITE_DBCONFIG_DEFENSIVE because the engine is already defensive here. | ||
| /// If a writable_schema unlock is ever added, it must be gated on a | ||
| /// per-connection defensive flag first (see turso_db_config_int in | ||
| /// bindings/c/src/lib.rs). |
|
|
||
| #define SQLITE_CANTOPEN 14 | ||
|
|
||
| #define SQLITE_CONSTRAINT 19 |
There was a problem hiding this comment.
It seems like you just added these constants in sqlite3.h, but not in the Rust code? The way this works is that you define constants in lib.rs and the cbindgen the header.
There was a problem hiding this comment.
Moved them into lib.rs as pub consts next to the existing result-code constants; the header mirrors them. (Values are verified against sqlite.h.in.)
| }; | ||
| inner.authorizer = callback.map_or(0, |f| f as usize); | ||
| inner.authorizer_ctx = context as usize; | ||
| SQLITE_OK |
There was a problem hiding this comment.
We set the authorizer, but never use it? This is not a good idea because callers have no way to know it just does not work, better just return SQLITE_MISUSE always or something if you just want the stub. An even better solution is to implement the functionality.
There was a problem hiding this comment.
Agreed — it now refuses with SQLITE_ERROR and the stored-callback fields are gone. Storing without invoking would also silently drop enforcement callers rely on (PHP routes its open_basedir checks through the authorizer). PHP discards this return value when installing its authorizer at open (verified in php-src), so refusing doesn't break it. Real implementation once core grows a prepare-time authorization hook.
| core::arch::naked_asm!("jmp {}", sym turso_sqlite3_db_config_va); | ||
| #[cfg(any(target_arch = "arm", target_arch = "aarch64"))] | ||
| core::arch::naked_asm!("b {}", sym turso_sqlite3_db_config_va); | ||
| } |
There was a problem hiding this comment.
What a weird way to do this. You can just drop sqlite3_db_config() from the Rust file altogether and use the symbol from the C code. Btw rust-lang/rust#155697 eventually solve this
There was a problem hiding this comment.
Using the C symbol directly was the first version — CI rejected it with mold: error: undefined symbol: sqlite3_db_config: rustc only exports Rust items from cdylibs (version script on ELF, exported-symbols list on Apple), so a symbol defined in a linked C object never makes it into the dynamic symbol table. Until rust-lang/rust#155697 lands, the exported symbol has to be a Rust item, and a naked tail-jump is the only stable-Rust way to forward a variadic call frame. Added a comment on the trampoline saying it's deletable (and the C function renamable to sqlite3_db_config) once that PR lands.
…te3_extended_result_codes All three were panicking stubs or missing; a Rust panic across the extern "C" boundary aborts the host process, and PHP's ext/sqlite3 calls sqlite3_set_authorizer unconditionally when opening a connection. - sqlite3_set_authorizer: refused with SQLITE_ERROR. turso_core has no prepare-time authorization hook, so a stored callback would never be invoked and callers (e.g. PHP, which routes its open_basedir checks through the authorizer) would silently lose the enforcement they registered for. PHP discards this return value at open, so refusing does not break it. To be implemented for real once core grows an authorization hook. - sqlite3_db_config: variadic entry point in a new C shim (varargs.c) since C variadics are not expressible in stable Rust and variadic arguments use a different ABI from named arguments on Apple arm64. Decodes the (int, int*) shape shared by boolean ops and forwards to a Rust backend; ops with other shapes are rejected without reading the va_list. SQLITE_DBCONFIG_DEFENSIVE is refused with SQLITE_ERROR rather than accepted-and-ignored: the engine is unconditionally defensive today (sqlite_schema DML is rejected with no writable_schema unlock, PRAGMA schema_version=N is a deliberate no-op, PRAGMA journal_mode=OFF is unsupported, and there are no shadow tables), so there is nothing to toggle and a caller that checks the return is not misled. Accepting DEFENSIVE becomes possible once each of those features gains a per-connection defensive gate. PHP ignores this return value. Other ops return SQLITE_ERROR. - The exported sqlite3_db_config symbol is a naked-function trampoline that tail-jumps to the C implementation: rustc exports only Rust items from cdylibs (version script on ELF, exported-symbols list on Apple), so the public symbol must be a Rust item. The tail jump hands the untouched variadic call frame to the C function, and the sym reference pins varargs.c's object into the link. Deletable once rust-lang/rust#155697 lands. - sqlite3_extended_result_codes: toggles the connection's err_mask. The default mask now matches SQLite (extended codes disabled), and sqlite3_extended_errcode no longer masks its result: it reports the extended code regardless of the setting. - Constants (authorizer return/action codes, SQLITE_DBCONFIG_*) are defined in lib.rs and mirrored in the header, all verified numerically against SQLite's sqlite.h.in. Tests in sqlite3_tests.c cover the registration contract for the authorizer (accepted by SQLite, refused by Turso, execution unaffected either way), the DEFENSIVE set/query/reset cycle on SQLite and refusal with untouched out-pointer on Turso including the exact NULL-out-pointer call PHP makes at open, unknown-op rejection, and errcode masking around a UNIQUE violation with extended codes off, on, and off again. The same binary passes linked against both turso_sqlite3 and libsqlite3. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@penberg — disclosure: the review-thread replies above (and the code changes addressing them) were written by Claude, Glauber's coding agent, not by Glauber himself. He is currently at Disneyland and has instructed me to yield to you in everything — if any of my pushback (e.g. on the db_config trampoline) doesn't convince you, say the word and I'll do it your way. All six comments are addressed in the latest push; ready for another review round. |
2 participants
Implements three
sqlite3_*entry points inbindings/cthat were panicking stubs or missing. A Rust panic across theextern "C"boundary aborts the host process, and PHP'sext/sqlite3callssqlite3_set_authorizerunconditionally when opening a connection — so today, any PHP-style consumer dies on its firstopen().What's in here
sqlite3_set_authorizer— full six-argument callback signature; registers, replaces, or clears (NULL) the connection's authorizer. The callback is stored but not consulted: turso_core has no prepare-time authorization hook yet, so statements behave as if it returnedSQLITE_OK. Enforcement (DENY/IGNORE) comes with a future core hook.sqlite3_db_config— a genuinely variadic entry point, which stable Rust cannot define (c_variadicis nightly-only), and which cannot be faked with fixed arguments: on Apple arm64, variadic arguments are passed on the stack while named arguments use registers, so a fixed-arg callee reads garbage. It lives in a new C shim (src/varargs.c, built viacc), decodes the(int, int*)shape shared by the boolean ops, and forwards to a non-variadic Rust backend. Ops with other shapes are rejected without reading the va_list.SQLITE_DBCONFIG_DEFENSIVEis refused withSQLITE_ERRORrather than accepted-and-ignored, so a caller that checks the return cannot be misled. The protections DEFENSIVE stands for are unconditionally on in turso_core today:sqlite_schemaDML is rejected with nowritable_schemaunlock,PRAGMA schema_version=Nis a deliberate no-op,PRAGMA journal_mode=OFFis unsupported, and there are no shadow tables. The core sites that keep those guarantees now carry comments marking them as flip points — if any of those features ever lands, it must gate on a per-connection defensive flag, at which point the C API can switch to accept + enforce. (PHP ignores this call's return value, verified against php-src.)sqlite3_extended_result_codes— toggles the connection'serr_mask. Two fixes fell out: the default mask now matches SQLite (extended codes disabled by default), andsqlite3_extended_errcodeno longer masks its result — it reports the extended code regardless of the setting, as documented.Header — corrected the
set_authorizer/db_configprototypes, addedextended_result_codes, authorizer return + action codes,SQLITE_DBCONFIG_*, andSQLITE_CONSTRAINT. All constants verified numerically against SQLite'ssqlite.h.in/sqliteInt.h(90/90 match).build.rs — the shim object is kept alive via a
#[used]reference (nothing else references it, so archive linking would otherwise drop it) and exported explicitly on Apple targets, where rustc's exported-symbols list would demote linked-in C symbols to local.Tests
New cases in
tests/sqlite3_tests.c, all written so the same binary passes linked against turso_sqlite3 and real libsqlite3 (as CI's c-compat workflow does):🤖 Generated with Claude Code